Skip to content

doc: update contributing guidelines wrt pre-commit/devtool #5204

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from

Conversation

DakshinD
Copy link
Contributor

@DakshinD DakshinD commented May 8, 2025

Changes

Point external contributors to use devtool instead of unmaintained pre-commit script to check code
quality/style in CONTRIBUTING.md. This is due to pre-commit script not being maintained anymore.

Reason

Addresses issue #5201

License Acceptance

By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license. For more information on following Developer
Certificate of Origin and signing off your commits, please check
CONTRIBUTING.md.

PR Checklist

  • I have read and understand CONTRIBUTING.md.
  • I have run tools/devtool checkstyle to verify that the PR passes the
    automated style checks.
  • I have described what is done in these changes, why they are needed, and
    how they are solving the problem in a clear and encompassing way.
  • I have updated any relevant documentation (both in code and in the docs)
    in the PR.
  • I have mentioned all user-facing changes in CHANGELOG.md.
  • If a specific issue led to this PR, this PR closes the issue.
  • When making API changes, I have followed the
    Runbook for Firecracker API changes.
  • I have tested all new and changed functionalities in unit tests and/or
    integration tests.
  • I have linked an issue to every new TODO.

  • This functionality cannot be added in rust-vmm.

Point external contributors to use devtool instead
of unmaintained pre-commit script to check code
quality/style.

Signed-off-by: Dakshin Devanand <[email protected]>
Copy link
Contributor

@Manciukic Manciukic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution! The contribution guideline doc update looks good to me, but we should add a warning in the existing pre-commit script and mark it as deprecated.
We should also probably spin up a new issue to add the git-secrets check to the devctr integration tests.

Comment on lines +74 to +76
> **Note:** The legacy rusty-hook pre-commit script has been deprecated and is
> unmaintained. Please use `devtool` for formatting, linting, and build checks
> during your development cycle.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add a warning in the original script as well so that we may remove it in the future?

@Manciukic Manciukic self-assigned this May 21, 2025
@Manciukic Manciukic added Status: Awaiting author Indicates that an issue or pull request requires author action Status: WIP Indicates that an issue is currently being worked on or triaged labels May 21, 2025
@Manciukic Manciukic linked an issue May 21, 2025 that may be closed by this pull request
Deprecates pre-commit hook and adds warning to the user,
due to switch to devtool.

Signed-off-by: Dakshin Devanand <[email protected]>
@Manciukic Manciukic mentioned this pull request May 28, 2025
10 tasks
@roypat
Copy link
Contributor

roypat commented May 28, 2025

Hey, sorry, I wasn't aware someone else was already working on this. I ended up closing out on #5201 as part of #5225, so I'll go ahead an close this PR, as its no longer needed now. Thanks for your contribution anyway!

@roypat roypat closed this May 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Awaiting author Indicates that an issue or pull request requires author action Status: WIP Indicates that an issue is currently being worked on or triaged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update contribution guidelines wrt pre-commit hook
3 participants